feat: migrate implement-ticket and babysit-pr to the schema 1.3 review-result contract - #86
Merged
Merged
Conversation
…w-result contract - Bundle the canonical review-suite contract (CONTRACT.md, both schemas, validate.py) into `skills/implement-ticket/references/review-suite/` and `skills/babysit-pr/references/review-suite/`, and wire `just sync-contracts` and the shared bundled-contract test to keep both byte-identical to the canonical source. - Add `scripts/review_gate.py` to each caller: a thin consumption check (not a reviewer) that runs the bundled schema validator against a `review-code-change` result and additionally binds it to the caller's exact current head/comparison-base SHA, rejecting stale `schema_version` values, malformed shape, non-`aggregate` results, non-`clean` verdicts, and incomplete or stale `lens_executions`. - Update `implement-ticket`'s review-and-merge gate and babysit-pr-handoff prose, and `babysit-pr`'s final gate and post-fix re-review prose, to require validating every review result against the bundled contract before consuming it, and to state explicitly that green CI or connector approval never substitutes for that validated result. - Add focused tests for the new gate (stale/unsupported schema version, malformed shape, blocked/changes_required verdicts, incomplete/stale lens executions, wrong-candidate binding, current-clean acceptance) and new eval cases proving each caller blocks publication/readiness on the same conditions, plus a case proving an already-current clean aggregate needs no extra invented review cycle. - #55 (epic #49) requires both meta-skills to correctly consume, validate, and reject the review-result contract at its final version (schema 1.3, from #51-#53) without reimplementing lens sequencing, exploration, or aggregation, which stay owned by `review-code-change`.
…mechanism ## Summary - Add `review-suite/scripts/review_gate.py` and `review-suite/scripts/tests/test_review_gate.py` as the canonical source for the caller-side review-result consumption check, resolving `validate.py` in either the canonical or installed skill layout. - Extend `just sync-contracts` to copy both into `implement-ticket` and `babysit-pr`'s `scripts/` directories, replacing the two hand-duplicated copies with synced ones. - Extend `review-suite/scripts/tests/test_bundled_contracts.py` with a `GATE_BUNDLING_SKILLS`/`GATE_CANONICAL_FILES` check so future drift between the two copies fails a test the same way the four sibling bundled files already do. ## Why - `review-code-change`'s own solution-simplicity and code-simplicity lenses found the two per-skill `review_gate.py`/test copies were byte-identical hand duplicates with no drift protection, unlike the four sibling bundled files this same change already protects with the established canonical-source-plus-sync-plus-drift-test pattern. This closes that gap by reusing the existing mechanism instead of inventing a new one.
…ation fix - Add the missing CHANGELOG bullet for commit `1dc2a492` (the review_gate.py canonicalization fix) and backfill its SHA now that it is superseded. - Backfill the SHA onto the preceding `feat: migrate implement-ticket and babysit-pr...` entry. - A re-review of the #55 caller-migration candidate found the fix commit changed caller-consumed behavior but never recorded it in CHANGELOG.md, breaking this repository's otherwise-unbroken daily-changelog convention (AGENTS.md Git Workflow).
## Summary - Correct the two CHANGELOG.md SHAs for the review_gate.py fix and the caller migration commit, which changed identity when this branch was rebased onto the current origin/main (a competing PR merged first, forcing a rebase to resolve a real CHANGELOG.md conflict). - Every cited SHA under `## 2026-07-29` is now verified with `git merge-base --is-ancestor` against the current HEAD. ## Why - A fresh review-code-change cycle, run after the rebase, found the two SHAs backfilled before the rebase pointed at orphaned pre-rebase commit objects rather than the real in-history commits, which would have broken traceability once the stale remote ref is superseded.
shaug
force-pushed
the
scott/55-caller-contract-migration
branch
from
July 29, 2026 14:10
ce76e65 to
51cc734
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
implement-ticketandbabysit-prto consume the final versionedreview-result contract (schema 1.3, established by #51 -> #52 -> #53), and
proves their existing full-review/current-head gates reject stale, malformed,
unsupported-version, blocked, changes-required, and incomplete evidence
without reimplementing lens sequencing, exploration, or aggregation.
review-suitecontract (CONTRACT.md, both schemas,validate.py) intoskills/implement-ticket/references/review-suite/andskills/babysit-pr/references/review-suite/, wired throughjust sync-contractsand the shared byte-identical-bundle drift test.review-suite/scripts/review_gate.py(+ its test), syncedthe same way into each caller's
scripts/: a thin consumption check thatruns the bundled schema validator and additionally binds a
review-code-changeresult to the caller's exact current head/base,rejecting stale
schema_version, malformed shape, non-aggregateresults,non-
cleanverdicts, and incomplete/stalelens_executions.implement-ticket's review-and-merge gate / babysit-pr handoff, andbabysit-pr's final gate and post-fix re-review prose, to require thisvalidation before consuming any review result, and state explicitly that
green CI or connector approval never substitutes for it.
test_review_gate.py) and new eval cases in both skillsproving each rejection scenario, plus one case proving an already-current
clean aggregate needs no extra invented review cycle.
Acceptance criteria (from #55)
behavior, so migration cannot regress it.
implement-ticketrejects stale, malformed, unsupported-version,failed-validation, blocked, changes-required, or incomplete review
evidence.
babysit-prrejects the same invalid evidence and cannot become readythrough green CI or connector state alone.
review-code-change; callers do not manually invoke individual lenses(unchanged, pre-existing prose, preserved).
unrelated drift preserves the existing risk-based behavior (unchanged,
pre-existing prose, preserved).
private evaluator/explorer data.
progresses without an extra invented review cycle.
contract.
just format,just lint, andjust testpass.Review history
Three
review-code-changecycles were run against this candidate lineage:501c4375d8aad136a62836c9f435441efe0b8315vs basea247d47d...:changes_required(onestrong_recommendation: two hand-duplicatedreview_gate.py/test copies with no drift protection).1dc2a492bd3b54961b264dec84b4f00cefe5b4e3vs basea247d47d...:prior finding independently verified resolved;
changes_requiredagain(one new
strong_recommendation: the fix commit was missing itsCHANGELOG.md entry).
ce76e6581213c3eda1f180a99b9b6edd819ae3efvs basea247d47d...:cleanacross all three lenses (solution simplicity, correctness,code simplicity), zero findings.
Out of scope / untouched
review-suite/evals/baseline/v1/andreview-suite/evals/v2/(frozen/in-progress artifacts from Curate the scored corpus and capture the frozen v1 review baseline #58/Use the v1 baseline to finalize and preregister the review v2 graph #59 and a separate recheck task) — confirmed
untouched via
git diff --statagainst those paths.carve-changesetswas read only for its cognitive-load guardrails, neverinvoked (candidate is not oversized: the real novel reviewable surface is a
few hundred lines, the rest is mechanically-synced, tested-identical
bundles).
cleanup ownership are unchanged.
Closes #55